Skip to content

Conversation

mashhurs
Copy link
Collaborator

Description

This PR adds a PR backporter which we need to backport changes to designated base branches when we merge PRs to target branches.

…base branches when we merge PRs to target branches.
@mashhurs mashhurs requested a review from Copilot December 12, 2024 17:58
@mashhurs mashhurs self-assigned this Dec 12, 2024
@mashhurs mashhurs requested a review from donoghuc December 12, 2024 17:59
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 suggestions.

Comments skipped due to low confidence (2)

devtools/backport:114

  • The commit_hashes variable should be joined with a space, not assigned a string with a space. Use 'commit_hashes = " ".join(args.commit_hashes)' instead.
commit_hashes = "{}".join(args.commit_hashes)

devtools/backport:212

  • The get_github_token function should catch specific exceptions and provide a clear error message. Use 'except FileNotFoundError:' instead.
except:

@mashhurs mashhurs linked an issue Dec 12, 2024 that may be closed by this pull request
Not bad Copilot - apply suggestion of version strip and writing token into a file.

Co-authored-by: Copilot <[email protected]>
@donoghuc
Copy link
Member

Is there a reason we can just use an off the shelf solution for backport? https://github.com/tibdex/backport seems to be the most popular.

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

cc @mashhurs

@mashhurs
Copy link
Collaborator Author

Is there a reason we can just use an off the shelf solution for backport? https://github.com/tibdex/backport seems to be the most popular.

We have a common script in LS core for this purpose: https://github.com/elastic/logstash/blob/main/devtools/backport
I am open to use https://github.com/tibdex/backport or any other new tool but it may require extra processes/effort. For example, our devtools backlporter is fully automated, creates label and attaches to the PR. As I do see with https://github.com/tibdex/backport, we need to manually create label or automate it by converting current python code into nodejs?! (just checked high level details). tibdex/backport seems simplified interactions with the GH API, where we are directly using them in our backporter.

Do you have any concerns with the LS core devtool/backporter?

Honestly, I haven't seen any pain points so far with LS core backporter. If we have, I would support migrating all LS core and other plugin backporters (if we have) altogether.

Copy link
Member

@donoghuc donoghuc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only concern apart from general maintainability issues using our own script vs consuming a popular one is copying around files that need updates in the body https://github.com/elastic/logstash-filter-elastic_integration/pull/203/files#diff-38840c5e92fffb039a64f7632661853ab0075df02cb3f3dc123f1de961936a27R100

I'm fine with just doing this for now while we evaluate benefits of using a pre built action :)

@mashhurs
Copy link
Collaborator Author

My only concern apart from general maintainability issues using our own script vs consuming a popular one is copying around files that need updates in the body https://github.com/elastic/logstash-filter-elastic_integration/pull/203/files#diff-38840c5e92fffb039a64f7632661853ab0075df02cb3f3dc123f1de961936a27R100

I'm fine with just doing this for now while we evaluate benefits of using a pre built action :)

Ah from that perspective makes sense. Let me create a placeholder to push forward. For now I am not seeing its urgency or broken features, it is nice to have tech dept!

@mashhurs mashhurs merged commit 22092f7 into elastic:main Dec 12, 2024
2 checks passed
@mashhurs mashhurs deleted the add-pr-backporter branch December 12, 2024 20:19
@mashhurs
Copy link
Collaborator Author

@logstashmachine backport 8.x

1 similar comment
@mashhurs
Copy link
Collaborator Author

@logstashmachine backport 8.x

@donoghuc donoghuc mentioned this pull request Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Automate backportering

3 participants